-
Notifications
You must be signed in to change notification settings - Fork 73
fix: remote flag not working for preview command's cache population #888
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
🦋 Changeset detectedLatest commit: d0cff66 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
Thanks for the fix @james-elicx @dario-piotrowicz could you please take a look as you worked on the remote bindings. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me 🙂
Although, tangential to the changes here, long term I would say that ideally remote bindings should be used instead of running the worker with --remote
(unless there's a benefit/reason to do so?)
I think the plan is to remove |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@james-elicx I added a few minor comments.
The one most important thing is to add the "r" alias for "remote" IMO.
Thanks!
Sorry for the delay in the review, please see my comments. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks James!
When passing the
--remote
flag to ourpreview
command, it would fail to properly populate the cache due to not targeting the remote bindings and missing a--preview
flag for Workers KV / D1.fixes #887
Changes:
Tested that cache population was working as expected with the following commands for the
d1-tag-next
example:opennextjs-cloudflare preview --config wrangler.e2e.jsonc
opennextjs-cloudflare preview --config wrangler.e2e.jsonc --remote
opennextjs-cloudflare deploy --config wrangler.e2e.jsonc